Multi-replica: move all cross-request state into Postgres - #301
Merged
Conversation
First step of the multi-replica epic: the transport, plus the smallest subsystem that needs it. Six settings singletons are cached in process memory so hot paths never touch the database. That invalidation was done by the request that wrote the row, which was sufficient with one process and leaves every other replica serving stale config with more than one. pg_bus publishes descriptors over LISTEN/NOTIFY from *inside* the writing transaction. Postgres then delivers only on COMMIT and discards on rollback, which is the same guarantee domain_events hand-builds for the local process — and the reason this epic is on Postgres rather than Redis. pg_listener holds one dedicated asyncpg connection per replica (not a pooled checkout: LISTEN is connection state) with a jittered backoff reconnect, and splits the socket from the work so a slow handler cannot stall asyncpg's protocol callbacks. Payloads are descriptors, never rendered content: NOTIFY caps at 8000 bytes, so each replica re-reads the committed row instead. Delivery is at-most-once, so config_sync re-reads every scope on reconnect — skipping the first connect, where the startup loaders have just done it and refreshing OIDC would drop the Authlib registration the lifespan performed moments earlier. Behaviour is unchanged for a single replica. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Domain events carried live ORM instances, which cannot cross a process boundary — so a second replica's clients would simply never hear about anything the first one did. Every event now carries ids and scalars, and handlers read the rows themselves. On the recording replica those reads are identity-map hits; on a peer they are the point. It also removes a latent staleness bug the seam had been working around: the lifecycle CAS runs with synchronize_session=False, so a carried instance was a snapshot of pre-transition state, while a row read after the commit is authoritative. Publication is issued from before_commit rather than beside dispatch. Postgres holds a NOTIFY until COMMIT and discards it on rollback, so a peer's copy of a frame is exactly as trustworthy as the local one, and the "committed but never announced" window cannot reopen. record() now refuses an event with an unset id, since a row no replica can read back is not announceable. The origin keeps projecting in-band and skips its own descriptors. Frame-building is therefore identical on both paths, and the savepoint-isolated suite still exercises projection end to end — a transaction that never truly commits can never deliver a NOTIFY. Handlers that *act* rather than project (arming triggered comms, the #218 cursor re-arm) are marked subscribe_local: rendering a frame on every replica is the point, arming a timer on every replica fires it once per replica. Both become transactional enqueues in step 3. OIDC role downgrades announce themselves too — authorization is per-connection state on whichever replica holds the socket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Inject releases, triggered communications, LLM pipelines and the audit sweep were asyncio tasks holding their state in dicts. That is why a restart mid-exercise lost pending work (#211) and why the app could not run a second replica. They are procrastinate jobs now. The reason for procrastinate over Celery or ARQ is transactional enqueue: the job row is INSERTed by procrastinate_defer_jobs_v1 on the caller's own session, and its insert trigger's pg_notify is held by Postgres until COMMIT. So the job exists if and only if the state change that warranted it committed. A Redis-backed queue would make the split between the two stores permanent architecture, and "committed but never enqueued" a permanent bug class. That property is what lets three hand-built coordination mechanisms go: - Triggered comms move from a post-commit subscriber into release_inject's transaction. #211 was exactly the gap between those two points. - The #218 cursor re-arm moves into the response's transaction, which deletes the rearm_requested flag and _consume_rearm_or_deregister along with the await-free window they had to coordinate in. - The LLM in-flight sets become queueing locks, so a manual re-trigger on another replica can no longer buy a second provider call. Nothing cancels jobs any more. Pausing, completing, or releasing early leaves them in place to fire, re-read durable state, and no-op or re-defer — safe because the CAS release and the (exercise_id, trigger_key) unique insert already made replays no-ops. Losing a job is the only outcome that hurts, and that is the one the enqueue rules out. The retention sweep becomes a periodic task, so one replica purges each night rather than every replica purging concurrently. The worker runs in-process, so a deployment is still one container. psycopg arrives with procrastinate (its worker cannot speak asyncpg); the binary wheel carries libpq, so the image needs no system packages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The login, registration, and password-reset limiters kept their sliding windows in process memory, so each replica enforced a private allowance: three replicas behind a load balancer multiplied the effective login limit by three, and an attacker who kept reconnecting got the multiplier for free. Counters are now rows in an unlogged table — one per counted attempt, so the window still slides rather than resetting in a block. A fixed bucket would let an attacker spend the whole allowance at the end of one window and again at the start of the next. UNLOGGED is the reason a write per login attempt is acceptable: the rows never reach the WAL and Postgres truncates them after an unclean shutdown, which at worst forgives some in-progress lockouts. Attempts are counted on their own connection rather than the request session. A failed login records a strike and then raises, and a strike a rollback could erase is not a strike. The opportunistic in-process sweep becomes an hourly maintenance job. The old one ran on a read path, so rows for a key nobody consulted again stayed forever — and the keys are exactly the ones an attacker can mint at will (a spoofed X-Forwarded-For, an arbitrary email). Thresholds stay in memory: they are configuration, and already reach every replica over the config bus with the settings they belong to. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes the epic: the constraint is gone, so the manifests and the ~20 places that documented it have to stop saying otherwise. Every replica migrates on startup, so a rollout would run several `alembic upgrade head` concurrently against one database — two processes creating the same table is a crash loop, not a race you can retry past. env.py takes a Postgres advisory lock first; the losers find the schema at head and apply nothing. That keeps the self-contained deploy story (one manifest, compose parity) rather than splitting migrations into a separate Job. Scaling out ships as an opt-in overlay rather than a change to the base. The remaining blocker is storage, not state: the uploads PVC is ReadWriteOnce and a PVC's access modes cannot be changed in place, so flipping the base would break every existing install's upgrade path. The overlay sets replicas: 2, a RollingUpdate that keeps the old pod serving until the new one is ready, and an RWX uploads claim. Docs record two things an operator can only learn the hard way: a transaction-mode pooler silently breaks LISTEN/NOTIFY (live updates stop for some clients while everything else looks healthy), and a migration must now be forward-compatible across one release, because a rolling update serves the previous version against the new schema. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The advisory lock was issued before context.configure, which autobegan a transaction on the connection that Alembic then never committed — so every migration silently rolled back and a fresh database ended up with no schema at all. The app would have failed on first start, and only on a first start, which is the worst possible place to find it. An xact lock taken inside context.begin_transaction() serialises the same way, releases however the migration ends, and cannot strand a transaction. Adds the tests that caught it: the chain applied to a genuinely empty database, `alembic check` against it (which also pins the include_name filter that keeps procrastinate's library-owned tables out of autogenerate), and that rate_limit_hits really is UNLOGGED. Nothing else in the suite runs Alembic — it builds its schema from the models — so a broken migration had no way to fail a test before now. env.py also honours an explicitly configured sqlalchemy.url, since settings are bound at import and could not otherwise be pointed at the throwaway database these tests create. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every other listener test drives a stubbed connection, so a mistake in the parts asyncpg owns — the subscription, the callback signature — would have passed the suite and failed in production. This runs the real path: connect, LISTEN, callback, queue, consumer, handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two findings from reviewing the branch. The sample loader deferred its release jobs and then relied on the conditional release_inject call afterwards to commit them — so a demo whose start inject did not match the member's group rolled the jobs back silently at session teardown. It commits its own enqueue now. The bulk re-enqueue paths (resume, startup reconciliation) wrote a fresh job set unconditionally, so every pause cycle and every replica restart stacked a duplicate set per active exercise. They now skip anything that already has a live todo job. Skipping a stale job is safe because a stale deadline is only ever early — pauses extend deadlines, never shorten them — and an early-firing job re-defers itself with the recomputed remaining time. Only todo counts as live: a doing job may have read its state before the change that prompted the re-enqueue, and a duplicate against it is a cheap no-op where a skipped enqueue against a job about to stand down would strand the schedule. Also adds the test the queue was missing entirely: a real worker picking up and running a committed job — connector DSN, NOTIFY wake-up and task deserialisation included. Everything else invokes task functions directly, so a wiring mistake there would have passed the suite and hung silently on the first deploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The UI suite runs pytest against a server that has already migrated their shared database, so the harness's apply_schema met procrastinate's objects and failed every test at fixture setup — procrastinate's DDL, unlike create_all, does not skip what already exists. Local runs never see this because the testcontainer database is fresh. apply_schema now checks for the schema first and no-ops when present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The limiter-reset fixture called asyncio.run at every teardown, and the Playwright tests execute while the session-scoped loop is live — where asyncio.run refuses outright. Every UI test errored at teardown in CI; locally invisible, because test_ui never runs without a server on :8765. The fixture now deletes over a synchronous psycopg connection, which involves no loop at all — the same reasoning that already keeps every autouse fixture in this file sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the #213 epic on the all-Postgres design settled in the issue thread: LISTEN/NOTIFY for pub/sub, procrastinate for durable scheduling, an unlogged table for rate limits — no Redis, no broker.
The property everything hangs on is the transactional boundary. Every cross-replica announcement and every job enqueue is issued inside the transaction that makes it true: Postgres holds a NOTIFY until COMMIT and discards it on rollback, and procrastinate's defer function runs on the caller's own session. "Committed but never broadcast" and "committed but never enqueued" — the bug class behind #211, #212, and #218 — stop being representable, and the hand-built coordination those fixes needed (post-commit comm arming,
rearm_requested+_consume_rearm_or_deregister) is deleted rather than ported.By phase (one commit each)
pg_buspublishes id-only descriptors (NOTIFY caps at 8k);pg_listenerholds one dedicated asyncpg connection per replica with jittered-backoff reconnect; settings saves invalidate every replica's cache, with reconnect recovery.before_commit; the origin projects in-band and skips its own descriptors, so frame-building is identical on both paths and the savepoint-isolated suite still exercises it end to end. OIDC role downgrades close sockets on every replica.rehydrate_schedules(), and the retention loop are deleted. Worker runs in-process per replica.k8s/overlays/multi-replica(base staysreplicas: 1— the uploads PVC is RWO and access modes are immutable); ~20 stale single-replica doc references rewritten.Follow-up commits from self-review: the advisory lock moved inside Alembic's transaction (the session-lock version silently rolled back every migration on a fresh database — caught by the new
test_migrations.py, which is why that file exists), a missing commit in the sample loader, duplicate-job growth on restart/resume fixed with a live-job check, and end-to-end tests for the real listener connection and a real worker run.Notes for review
include_namesoalembic checkstays meaningful.Closes #213
🤖 Generated with Claude Code